Fix the aggregate version calculation #475
Conversation
* Rename Position to Revision in StreamEvent for clarify
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
src/Core/src/Eventuous.Persistence/AggregateStore/AggregatePersistenceExtensions.cs
Show resolved
Hide resolved
|
|
||
| var cancelCmd = new Commands.CancelBooking(new(cmd.BookingId)); | ||
| var secondResult = await Service.Handle(cancelCmd, cancellationToken); |
There was a problem hiding this comment.
Suggestion: Reuse existing ID object in test
| var cancelCmd = new Commands.CancelBooking(new(cmd.BookingId)); | |
| var secondResult = await Service.Handle(cancelCmd, cancellationToken); | |
| var cancelCmd = new Commands.CancelBooking(cmd.BookingId); | |
| var secondResult = await Service.Handle(cancelCmd, cancellationToken); |
There was a problem hiding this comment.
This won't work because cmd.BookingId is a string but CancelBooking constructor expects a BookingId instance which has no implicit conversion from string.
There was a problem hiding this comment.
Question:
This won't work because cmd.BookingId is a string but CancelBooking constructor expects a BookingId instance which has no implicit conversion from string.
Answer:
The user has identified a type mismatch issue where the AI suggested using cmd.BookingId (a string) directly in a constructor that expects a BookingId object instance. This is a case where the AI made an incorrect assumption about the code structure.
To help prevent similar issues in future suggestions, you can add specific guidance to the extra_instructions configuration:
[pr_code_suggestions]
extra_instructions = """
- When suggesting code changes involving domain objects or value objects, ensure type compatibility
- Do not assume implicit conversions between primitive types (like string) and domain objects
- If a constructor requires a specific type instance, verify the variable type matches before suggesting its use
"""However, it's important to note that this represents an inherent limitation of AI code analysis. The model may not always have complete context about custom type systems, domain objects, and their conversion rules in your codebase. While the extra_instructions can help guide the model toward better type awareness, it cannot guarantee the model will always correctly identify type mismatches in complex domain models.
For C# codebases with strong typing and domain-driven design patterns, you might also consider:
[pr_code_suggestions]
extra_instructions = """
- Respect strong typing in C# - do not suggest implicit conversions that don't exist
- When working with value objects or domain entities, verify constructor signatures match the provided arguments
"""This type of error is relatively rare but can occur when the AI lacks full context about your type system. The suggestions should still be reviewed critically, as recommended in the Understanding AI Code Suggestions documentation.
Relevant Sources:
Test Results 51 files + 34 51 suites +34 32m 0s ⏱️ + 21m 5s Results for commit 4868104. ± Comparison against base commit 05689c4. This pull request removes 5 and adds 14 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
User description
PR Type
Bug fix
Description
Fix aggregate version calculation for deleted streams
OriginalVersionfrom computed property to stored fieldRename
PositiontoRevisioninStreamEventfor clarityAdd test for stream deletion and recreation scenario
Refactor type registration method to extension method
Diagram Walkthrough
File Walkthrough
2 files
Fix version tracking for deleted streamsPass stream revision when loading aggregate8 files
Rename Position property to RevisionUpdate Position references to RevisionUpdate Position references to RevisionRefactor Register to extension methodUpdate Position references to RevisionUpdate Position references to RevisionUpdate Position references to RevisionUpdate Position references to Revision2 files
Pass version when loading test aggregateAdd test for stream deletion scenario